NFS client provisioner #6433

verwilst · 2018-07-02T21:37:31Z

What this PR does / why we need it:
Adds nfs-client-provisioner chart
Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:

k8s-ci-robot · 2018-07-02T21:37:33Z

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
If you have done the above and are still having issues with the CLA being reported as unsigned, please email the CNCF helpdesk: [email protected]

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

verwilst · 2018-07-02T21:42:46Z

I've signed the CLA in the meantime.

verwilst · 2018-07-02T21:45:40Z

Should obsolete #2779

verwilst · 2018-07-02T21:48:26Z

I've based some parts of the README from PR above though.

verwilst · 2018-07-02T21:54:46Z

I've mentioned Kubernetes 1.7+, but since I'm using quite recent api versions, maybe 1.9+ is more suited?

mlaccetti

A few things:

incubator has been somewhat deprecated, stable is a good place to start
kubernetes 1.9+
provisioner name probably should be empty by default, since afaik there is no "standard" for this

verwilst · 2018-07-03T16:11:19Z

Updated point 1 and 2. About 3. I feel like a 'default' is needed, so I took a look at nfs-server-provisioner. They do it as follows:

{{- define "nfs-provisioner.provisionerName" -}}
{{- if .Values.storageClass.provisionerName -}}
{{- printf .Values.storageClass.provisionerName -}}
{{- else -}}
cluster.local/{{ template "nfs-provisioner.fullname" . -}}
{{- end -}}
{{- end -}}

But i do not like the 'cluster.local/' prefix. I don't have a single cluster named 'cluster.local' for example so we shouldn't assume that's the name. Afaik there is no way to get the cluster name through Helm.
Any other ideas?

mlaccetti · 2018-07-03T17:24:59Z

cluster.local is the default DNS for k8s, so it isn't a particularly bad default.

mlaccetti

lgtm

kingdonb · 2018-07-06T21:52:36Z

What are the checks that are holding this patch up? Not sure how to read e2e CI and tide results

mlaccetti · 2018-07-06T23:05:47Z

@kingdonb Waiting on an approval - @unguiculus - can you take a quick peek?

unguiculus

A few more things:

Change incubator references to stable
Add heritage labels
Apply our RBAC best practices: https://github.com/kubernetes/helm/blob/master/docs/chart_best_practices/rbac.md

unguiculus · 2018-07-15T15:20:57Z

stable/nfs-client-provisioner/templates/deployment.yaml

@@ -0,0 +1,42 @@
+apiVersion: apps/v1beta2


… apply RBAC best practices, apps/v1

verwilst · 2018-07-15T21:25:12Z

Implemented all remarks, and fixed a storageClass issue while i was at it. :-)

verwilst · 2018-07-18T08:07:41Z

/assign @mattfarina

verwilst · 2018-08-15T19:45:51Z

/retest pull-charts-e2e

davidkarlsen · 2018-08-15T19:57:56Z

/lgtm

davidkarlsen · 2018-08-15T19:58:40Z

@unguiculus @prydonius
Any chance of getting this one in? It would be very nice to have it.

unguiculus · 2018-08-15T20:20:51Z

/lgtm

k8s-ci-robot · 2018-08-15T20:20:57Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: davidkarlsen, unguiculus, verwilst

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [unguiculus]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

* Add nfs-client-provisioner chart * Moved to stable, minimum version is 1.9+ * Change provisionerName default * Change incubator references to stable in README, Add heritage labels, apply RBAC best practices, apps/v1 * update values.yaml to serviceAccount changes * use helpers for storageclass provisionerName * actually use template for helper variable * Make allowVolumeExpansion and reclaimPolicy configurable * fullname everywhere, update readme and set to latest version * use helper for serviceAccountName * Update deployment.yaml * Update values.yaml * remove whitespace * use helper function for sa binding as well * add default nfs.server value for ci * add endpoints bindings * go to version 3.0.1 * add archiveOnDelete, sync clusterrole rules * fix README.md parameters Signed-off-by: Marek Bartik <[email protected]> Signed-off-by: Marek Bartik <[email protected]>

* Add nfs-client-provisioner chart * Moved to stable, minimum version is 1.9+ * Change provisionerName default * Change incubator references to stable in README, Add heritage labels, apply RBAC best practices, apps/v1 * update values.yaml to serviceAccount changes * use helpers for storageclass provisionerName * actually use template for helper variable * Make allowVolumeExpansion and reclaimPolicy configurable * fullname everywhere, update readme and set to latest version * use helper for serviceAccountName * Update deployment.yaml * Update values.yaml * remove whitespace * use helper function for sa binding as well * add default nfs.server value for ci * add endpoints bindings * go to version 3.0.1 * add archiveOnDelete, sync clusterrole rules * fix README.md parameters Signed-off-by: aba182 <[email protected]>

* Add nfs-client-provisioner chart * Moved to stable, minimum version is 1.9+ * Change provisionerName default * Change incubator references to stable in README, Add heritage labels, apply RBAC best practices, apps/v1 * update values.yaml to serviceAccount changes * use helpers for storageclass provisionerName * actually use template for helper variable * Make allowVolumeExpansion and reclaimPolicy configurable * fullname everywhere, update readme and set to latest version * use helper for serviceAccountName * Update deployment.yaml * Update values.yaml * remove whitespace * use helper function for sa binding as well * add default nfs.server value for ci * add endpoints bindings * go to version 3.0.1 * add archiveOnDelete, sync clusterrole rules * fix README.md parameters Signed-off-by: Jakob Niggel <[email protected]>

Add nfs-client-provisioner chart

5eb0c58

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 2, 2018

k8s-ci-robot requested review from foxish and lachie83 July 2, 2018 21:37

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jul 2, 2018

verwilst mentioned this pull request Jul 2, 2018

NFS client provisioner #2779

Closed

mlaccetti reviewed Jul 3, 2018

View reviewed changes

Moved to stable, minimum version is 1.9+

11c7984

Change provisionerName default

df53d67

mlaccetti approved these changes Jul 3, 2018

View reviewed changes

unguiculus suggested changes Jul 15, 2018

View reviewed changes

stable/nfs-client-provisioner/templates/deployment.yaml Outdated

@@ -0,0 +1,42 @@

apiVersion: apps/v1beta2

Copy link

Member

unguiculus Jul 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apps/v1

verwilst added 4 commits July 15, 2018 23:08

Change incubator references to stable in README, Add heritage labels,…

650853f

… apply RBAC best practices, apps/v1

update values.yaml to serviceAccount changes

48d0aed

use helpers for storageclass provisionerName

5ac4942

actually use template for helper variable

862463c

Make allowVolumeExpansion and reclaimPolicy configurable

0f40d9d

k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 15, 2018

unguiculus approved these changes Aug 15, 2018

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 15, 2018

k8s-ci-robot merged commit 0e6663c into helm:master Aug 15, 2018

verwilst deleted the nfs-client-provisioner branch August 15, 2018 20:49

This was referenced Aug 19, 2018

Decide on NFS Provisioner chart direction #4142

Closed

(nfs-pv-provisioner) umbrella chart for nfs-{client,server}-provisioner installed together #7252

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NFS client provisioner #6433

NFS client provisioner #6433

verwilst commented Jul 2, 2018

k8s-ci-robot commented Jul 2, 2018

verwilst commented Jul 2, 2018

verwilst commented Jul 2, 2018

verwilst commented Jul 2, 2018

verwilst commented Jul 2, 2018

mlaccetti left a comment

verwilst commented Jul 3, 2018

mlaccetti commented Jul 3, 2018

mlaccetti left a comment

kingdonb commented Jul 6, 2018

mlaccetti commented Jul 6, 2018

unguiculus left a comment

unguiculus Jul 15, 2018

verwilst commented Jul 15, 2018

verwilst commented Jul 18, 2018

verwilst commented Aug 15, 2018

davidkarlsen commented Aug 15, 2018

davidkarlsen commented Aug 15, 2018

unguiculus commented Aug 15, 2018

k8s-ci-robot commented Aug 15, 2018

NFS client provisioner #6433

NFS client provisioner #6433

Conversation

verwilst commented Jul 2, 2018

k8s-ci-robot commented Jul 2, 2018

verwilst commented Jul 2, 2018

verwilst commented Jul 2, 2018

verwilst commented Jul 2, 2018

verwilst commented Jul 2, 2018

mlaccetti left a comment

Choose a reason for hiding this comment

verwilst commented Jul 3, 2018

mlaccetti commented Jul 3, 2018

mlaccetti left a comment

Choose a reason for hiding this comment

kingdonb commented Jul 6, 2018

mlaccetti commented Jul 6, 2018

unguiculus left a comment

Choose a reason for hiding this comment

unguiculus Jul 15, 2018

Choose a reason for hiding this comment

verwilst commented Jul 15, 2018

verwilst commented Jul 18, 2018

verwilst commented Aug 15, 2018

davidkarlsen commented Aug 15, 2018

davidkarlsen commented Aug 15, 2018

unguiculus commented Aug 15, 2018

k8s-ci-robot commented Aug 15, 2018